feat(#1074): add Jira-source overlay to code harness - #1075
Conversation
|
🤖 Finished Review · ✅ Success · Started 10:11 PM UTC · Completed 10:53 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
|
Risk Assessment: elevated (3/5) DetailsLarge blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive Tier 1 to 2.63; high churn (4), extreme multi-author contention (5), and heavy fix/revert history (5) in Tier 2 produce 3.29; Tier 3 remains low (1.83) as this is an additive-only Jira overlay with clear issue scope and easy revertibility. Previous runRisk Assessment: elevated (3/5) DetailsLarge blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive Tier 1 to 2.63; high churn (4), multi-author contention (5), and heavy fix/revert history (5) in Tier 2 produce 3.14; Tier 3 remains low (1.67) as this is an additive-only Jira overlay with clear issue scope, yielding a weighted composite of 2.59 that rounds to 3 (elevated), consistent with the prior assessment. Previous run (2)Risk Assessment: elevated (3/5) DetailsLarge blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive Tier 1 up, while high churn (4) and multi-author contention (5) in Tier 2 reinforce elevated risk; Tier 3 is low (1.83) due to additive-only overlay with clear issue scope, but preserving prior score of 3 as Tier 1 signals are unchanged and Tier 2 high-signal dimensions support elevated level. Previous run (3)Risk Assessment: elevated (3/5) DetailsAnchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship (1), no security-sensitive files (1), and no dependency changes (1). Only 3 files changed since prior review — all documentation and configuration updates. Tier 2 churn and multi-author contention signals remain consistent. Tier 3: well-specified linked issue with clear acceptance criteria and CEL-guarded activation reduces deployment risk. Previous run (4)Risk Assessment: elevated (3/5) DetailsAnchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 11 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship (1), no security-sensitive files (1), and no dependency changes (1). Tier 2 confirms high churn and multi-author contention across existing files. Bulk of change is new Jira-source scripts, limiting regression risk. Tier 3: well-specified issue with clear acceptance criteria and CEL-guarded activation reduces deployment risk. Previous run (5)Risk Assessment: elevated (3/5) DetailsAnchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 5 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship (1), no security-sensitive files (1), and no dependency changes (1). Tier 2 shows high churn and multi-author contention across existing files. Bulk of change is new scripts, limiting regression risk. Tier 3: well-specified issue with clear acceptance criteria reduces ambiguity. Previous run (6)Risk Assessment: elevated (3/5) DetailsAnchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 5 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship, no security-sensitive files, and no dependency changes. High churn on harness/code.yaml and SKILL.md (Tier 2) adds historical risk, but the bulk of the change is new scripts. Well-specified issue with clear acceptance criteria (Tier 3) reduces ambiguity. Previous run (7)Risk Assessment: elevated (3/5) DetailsAnchored to prior score of 3. Tier 1 signals unchanged: large blast radius (5), 5 protected path changes (5), and CI workflow modification (4) drive risk up, offset by bot authorship, no security-sensitive files, and no dependency changes. Tier 2 confirms high churn and multi-author contention on existing files. Good issue alignment and largely additive change sustain the prior elevated assessment. Previous run (8)Risk Assessment: elevated (3/5) DetailsTier 1 signals unchanged from prior assessment — large blast radius with 5 protected path changes and CI workflow modifications drive risk up, while high churn and multi-author contention on existing files elevate Tier 2, but the change is largely additive new scripts by a bot with clear issue alignment and no security-sensitive changes, sustaining the prior elevated score. Previous run (9)Risk Assessment: elevated (3/5) DetailsLarge blast radius with 5 protected path changes and CI workflow modifications drive Tier 1 up, while high churn and multi-author contention on existing files elevate Tier 2, but the change is largely additive new scripts by a bot with clear issue alignment. |
ReviewFindingsHigh
Medium
Low
Next steps:
Previous runReviewFindingsHigh
Medium
Low
Next steps:
Previous run (2)ReviewFindingsMedium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsMedium
Low
Next steps:
Previous run (5)ReviewFindingsMedium
Low
Previous run (6)ReviewFindingsMedium
Low
Previous run (7)ReviewFindingsMedium
Low
Next steps:
Previous run (8)ReviewFindingsMedium
Low
Previous run (9)ReviewFindingsHigh
Medium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 10:55 PM UTC · Completed 11:04 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.57 |
- Update docs/code.md: Multi-forge section now describes overlays with CEL when: expressions instead of stale forge.<platform> references; FULLSEND_FORGE description references overlay env section; add FULLSEND_SOURCE_TRACKER to Variables table; document Jira-source overlay - Document intentional omission of existing-PR check in Jira pre-script (forge_list_prs_for_issue searches for #N closing keywords which do not apply to Jira issue keys) - Add ISSUE_NUMBER to Jira overlay sandbox env block (explicit dependency instead of relying on forge overlay merge) - Fix empty-issue-context test to actually exercise the empty-file guard (previously exited at CLI-failure branch before reaching -s check) - Correct JIRA_BASE_URL documentation: optional, not required - Add emoji prefix to Jira notice annotation for consistency - Clarify SKILL.md ISSUE_NUMBER wording for Jira flow Addresses review feedback on #1075
🔧 Fix agent — iteration 1 (bot-triggered)Addressed 9 of 13 review findings: updated stale documentation in docs/code.md (Multi-forge section, FULLSEND_FORGE description, added FULLSEND_SOURCE_TRACKER), documented intentional omission of existing-PR check for Jira flows, added ISSUE_NUMBER to Jira overlay sandbox env, fixed empty-context test coverage, corrected JIRA_BASE_URL documentation accuracy, added emoji prefix for naming consistency, and clarified SKILL.md wording. Disagreed with 4 findings: code-duplication (better as follow-up), code-organization (already documented), error-handling-idiom (style preference), and protected-path (informational). Fixed (9):
Disagreed (4):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 11:06 PM UTC · Completed 11:48 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
|
/fs-fix why FULLSEND_SOURCE_TRACKER and not FULLSEND_TRACKER (like triage)? |
|
🤖 Finished Fix · ✅ Success · Started 2:28 AM UTC · Completed 2:34 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.12 |
Align the Jira-source overlay env var with the established triage convention (FULLSEND_TRACKER) instead of inventing a new name. Addresses review feedback on #1075
🔧 Fix agent — iteration 2 (human-triggered)Renamed FULLSEND_SOURCE_TRACKER to FULLSEND_TRACKER across all 3 files (harness/code.yaml, skills/code-implementation/SKILL.md, docs/code.md) to align with the established triage harness convention. All pre-code-jira tests (12/12) and pre-code tests (33/33) pass. Fixed (3):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 2:36 AM UTC · Completed 2:56 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.07 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
|
🤖 Finished Fix · ✅ Success · Started 2:58 AM UTC · Completed 3:06 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.83 |
- Reorder Jira overlay after forge overlays in harness/code.yaml to fix ISSUE_URL env variable conflict under last-writer-wins merge semantics - Sanitize ISSUE_URL and JIRA_BASE_URL in GHA ::error:: workflow commands to prevent command injection via embedded newlines - Add warning log in SKILL.md when FULLSEND_TRACKER=jira but context file is missing, instead of silently falling through to forge APIs - Add "Testing code agent with Jira" section to LOCAL.md Addresses review feedback on #1075
🔧 Fix agent — iteration 3 (bot-triggered)Addressed 5 of 8 review findings. Fixed the medium-severity ISSUE_URL env variable conflict by reordering the Jira overlay after forge overlays. Hardened two GHA workflow command echo sites against injection. Added missing-file warning in SKILL.md and local testing docs. Disagreed with 3 findings: protected-path (informational), edge-case (documented intentional), code-duplication (out of scope). Fixed (5):
Disagreed (3):
Tests: passed
Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 3:08 AM UTC · Completed 3:50 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
|
I hit this in a live dispatch: the Jira overlay declares |
|
/fs-fix rebase on main and resolve merge conflicts |
|
🤖 Finished Fix · ✅ Success · Started 3:47 PM UTC · Completed 3:56 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.69 |
Replace the deprecated forge: block in harness/code.yaml with overlays: entries using CEL when: expressions. Add a Jira-source overlay guarded by event.source.system == "jira" that: - overrides pre_script with a Jira-aware pre-code-jira.sh that fetches the issue via fullsend issues get --tracker jira, validates the Jira URL and credentials, and writes the issue context to /tmp/jira-issue-context.json - copies the context file into the sandbox at /sandbox/workspace/.issue-context.json via host_files, keeping Jira credentials on the runner only - sets FULLSEND_SOURCE_TRACKER=jira in the sandbox so the agent knows to read the prepared context file Post-script selection remains based on runtime.forge so GitHub targets still push via GitHub and GitLab targets via GitLab. Both the Jira-source and target-forge overlays match and merge for a Jira-triggered run. Update the code-implementation skill to check for FULLSEND_SOURCE_TRACKER=jira and read .issue-context.json instead of calling gh issue view or the GitLab API. Add pre-code-jira-test.sh covering URL validation, credential checks, context file creation, and error paths for both Jira->GitHub and Jira->GitLab compositions. Closes #1074
- Update docs/code.md: Multi-forge section now describes overlays with CEL when: expressions instead of stale forge.<platform> references; FULLSEND_FORGE description references overlay env section; add FULLSEND_SOURCE_TRACKER to Variables table; document Jira-source overlay - Document intentional omission of existing-PR check in Jira pre-script (forge_list_prs_for_issue searches for #N closing keywords which do not apply to Jira issue keys) - Add ISSUE_NUMBER to Jira overlay sandbox env block (explicit dependency instead of relying on forge overlay merge) - Fix empty-issue-context test to actually exercise the empty-file guard (previously exited at CLI-failure branch before reaching -s check) - Correct JIRA_BASE_URL documentation: optional, not required - Add emoji prefix to Jira notice annotation for consistency - Clarify SKILL.md ISSUE_NUMBER wording for Jira flow Addresses review feedback on #1075
Align the Jira-source overlay env var with the established triage convention (FULLSEND_TRACKER) instead of inventing a new name. Addresses review feedback on #1075
- Reorder Jira overlay after forge overlays in harness/code.yaml to fix ISSUE_URL env variable conflict under last-writer-wins merge semantics - Sanitize ISSUE_URL and JIRA_BASE_URL in GHA ::error:: workflow commands to prevent command injection via embedded newlines - Add warning log in SKILL.md when FULLSEND_TRACKER=jira but context file is missing, instead of silently falling through to forge APIs - Add "Testing code agent with Jira" section to LOCAL.md Addresses review feedback on #1075
- Restore CODE_ALLOWED_TARGET_BRANCHES passthrough in top-level env.runner (env-var-contract-violation) - Replace FULLSEND_WORK_ITEM_URL with ISSUE_URL in SKILL.md — the sandbox only receives ISSUE_URL via the harness mapping (logic-error) - Use grep -qwF for cross-work-item ownership check to prevent substring false positives e.g. PROJ-1 matching PROJ-12 (injection-check-weakening) - Rename _sanitize_gha to _gha_sanitize and add ANSI-escape strip to match canonical definition (naming-inconsistency) - Apply _gha_sanitize to ::notice:: interpolation for defense-in-depth (GHA-workflow-command-injection) - Add REPO_FULL_NAME :? guard in pre-code-jira.src.sh (missing-validation) - Remove ISSUE_REF_KEYWORD intermediary to match test helper pattern (style-inconsistency) Addresses review feedback on #1075
docs/code.md described the code agent deriving the work-item key from FULLSEND_WORK_ITEM_URL, but that variable is runner-only — the Jira overlay maps it to ISSUE_URL in env.sandbox. The prior iteration fixed SKILL.md; this fixes the remaining reference in the design doc. Addresses review feedback on #1075
🔧 Fix agent — iteration 6 (human-triggered)Rebased branch onto origin/main and resolved 4 merge conflicts in harness/code.yaml and docs/code.md. GitLab overlay now uses providers/openshell pattern from main combined with our overlay CEL expressions. All branch-specific tests pass; 16 pre-existing test failures in post-retro-test.sh are unchanged from main. Fixed (1):
Tests: passedNext steps:
|
ab6e33c to
2bf2127
Compare
|
🤖 Finished Review · ✅ Success · Started 3:58 PM UTC · Completed 4:39 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $10.77 |
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
scripts/post-code.src.sh(file-level): Line 233 · [high] logic-error
Jira+GitLab composition will fail at runtime. When FULLSEND_FORGE=gitlab and event.source.system=jira, the Jira overlay (ordered last, last-writer-wins) sets ISSUE_URL to the Jira browse URL. The GitLab validation block calls forge_validate_issue_url on ISSUE_URL, which checks against the GitLab URL pattern. The Jira URL will never match, causing exit 1. Additionally, GITLAB_HOST derivation from ISSUE_URL would extract the Jira host instead of the GitLab host, breaking all subsequent GitLab API calls. EXTERNAL_WORK_ITEM is already set to true before this block, so it can be used to gate the behavior.
Suggested fix: When EXTERNAL_WORK_ITEM=true and FULLSEND_FORGE=gitlab, skip the forge_validate_issue_url check on ISSUE_URL and derive GITLAB_HOST from CI_SERVER_HOST instead.
skills/code-implementation/SKILL.md(file-level): Line 287 · [low] stale-reference
SKILL.md says the post-script will inject (#ISSUE_NUMBER) scope if missing, but the post-script now injects (WORK_ITEM_KEY) without # prefix for external-tracker runs. The text is incomplete.
skills/code-implementation/SKILL.md(file-level): Line 411 · [low] stale-reference
Branch naming convention says agent/- but does not account for external-tracker runs where the namespace is agent/<WORK_ITEM_KEY>-. Same pattern at lines 341 and 408.
|
🤖 Finished Fix · ❌ Failure (running pre-script: exit status 1) · Started 4:40 PM UTC · Completed 4:40 PM UTC Commit: Effort: high |
|
🤖 Finished Retro · ✅ Success · Started 5:11 PM UTC · Completed 5:28 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.69 |
Retro: PR #1075 — Jira-source overlay for code harnessTimeline: Issue #1074 opened Aug 27 by @ralphbean with detailed requirements. Code agent produced PR #1075 within ~14 minutes ($6.12). The PR went through 13 review cycles, 6 fix iterations (4 bot-triggered, 2 human-triggered), and required 5 manual commits from @ralphbean to resolve an infrastructure blocker. Merged Aug 31 after 4 days. Estimated total agent cost: $40–57. What went well:
What didn't go well:
Evidence for existing open issues:
Proposals filed
|
1 participant
Summary
forge:block tooverlays:with CELwhen:expressions, matching the pattern established in the triage harness (Port triage agent from forge to overlays method #1013)event.source.system == "jira") that fetches issue content viafullsend issues get --tracker jiraon the runner, writes it to a context file, and copies it into the sandbox without exposing Jira credentials.issue-context.jsonwhenFULLSEND_SOURCE_TRACKER=jirainstead of calling forge APIsDesign
The Jira overlay is ordered before the forge overlays so its
pre_scriptoverrides the default forge pre-script. Post-script selection remains based onruntime.forge, so a Jira→GitHub run still pushes and creates a PR through GitHub. Both the Jira-source and target-forge overlays match and merge for a Jira-triggered run (merge-all-matching behavior).Jira credentials (
JIRA_USER_EMAIL,JIRA_TOKEN,JIRA_BASE_URL) are scoped toenv.runneronly — they never enter the sandbox. The sandbox receives the issue content as a JSON file at/sandbox/workspace/.issue-context.jsonand aFULLSEND_SOURCE_TRACKER=jirasignal.Testing
pre-code-jira-test.shwith 12 test cases covering:JIRA_BASE_URLmismatch rejectedfullsendCLI failure handlingpre-code-test.shandpost-code-test.shtests pass unchangedmake check-bundle) passesCloses #1074
Post-script verification
agent/1074-jira-source-code-overlay)20bd058ec31d8cd9a6c34bb9d04b5247b0440fce..HEAD)